[rtl] Switch to ratified bitmanip extensions by thommythomaso · Pull Request #2459 · lowRISC/ibex

thommythomaso · 2026-07-13T12:51:40Z

No description provided.

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

marnovandermaas

Initial comments from me. Do you think it is worth squashing the commits that just remove things? I think it may be excessive to have 9 separate commits for removing instructions.

marnovandermaas · 2026-07-15T14:49:47Z

-            {7'b000_0100, 3'b100}, // pack
-            {7'b000_0100, 3'b111}, // packh


The zexth commit also remove pack and packh

marnovandermaas · 2026-07-15T14:50:22Z

                  5'b0_0000: alu_operator_o = ALU_SRL;   // Shift Right Logical by Immediate
                  5'b0_1000: alu_operator_o = ALU_SRA;   // Shift Right Arithmetically by Immediate
                  5'b0_1001: alu_operator_o = ALU_BEXT;  // Extract bit specified by immediate.
+                  // rori (Zbb, also imported by Zbkb): rotate right by immediate


Why does changing this comment complete Zbkb?

marnovandermaas · 2026-07-15T14:51:27Z

-* `RISC-V Bit-Manipulation Extension, version 1.0.0 <https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf>`_ and `version 0.93 (draft from January 10, 2021) <https://github.com/riscv/riscv-bitmanip/blob/master/bitmanip-0.93.pdf>`_
+* `RISC-V Bit-Manipulation Extension, version 1.0.0 <https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf>`_
+* `RISC-V Scalar Cryptography Extension, version 1.0.1 <https://github.com/riscv/riscv-crypto/releases/download/v1.0.1-scalar/riscv-crypto-spec-scalar-v1.0.1.pdf>`_ (Zbkb, Zbkx sub-extensions only)


The commit for this shouldn't be marked as RTL. Probably [doc,dv] would be better.

thommythomaso · 2026-07-15T14:55:39Z

Initial comments from me. Do you think it is worth squashing the commits that just remove things? I think it may be excessive to have 9 separate commits for removing instructions.

I have them as individual commits to help people review the changes. I would have squashed all into one after the review process. What do you think?

SamuelRiedel

Thanks for cleaning up the bitmanip @thommythomaso. I did a quite thorough review, but most of it should be easy to fix.

The only thing that is left is that we should update RISC-V DV to also support the latest ratified extensions. I saw that the ratified B extensions are supported already, but for the scalar crypto ones, we currently simply enable the Zbp still, right? I think this is good enough for now, but we should create an issue documenting the current state, like which instructions are emitted by riscv-dv and which aren't and what we would ideally want to then clean up the DV in a second step at least.

SamuelRiedel · 2026-07-20T08:52:59Z

  Ibex implements the Machine ISA version 1.12.
 * `RISC-V External Debug Support, version 0.13.2 <https://content.riscv.org/wp-content/uploads/2019/03/riscv-debug-release.pdf>`_
-* `RISC-V Bit-Manipulation Extension, version 1.0.0 <https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf>`_ and `version 0.93 (draft from January 10, 2021) <https://github.com/riscv/riscv-bitmanip/blob/master/bitmanip-0.93.pdf>`_
+* `RISC-V Bit-Manipulation Extension, version 1.0.0 <https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf>`_


Should we explicitly mention the subextensions? The "B" only stands for Zba, Zbb, and Zbs in the ISA string, but then the Zbc is also part of the bit manipulation extension. This is a bit confusing in the RISC-V spec already, but I would explicitly mention what we support already in this file here and in the list below.

SamuelRiedel · 2026-07-20T09:00:09Z


   * - **B**: Standard Extension for Bit-Manipulation Instructions
-     - 1.0.0 + 0.93 [#B_draft]_
+     - 1.0.0 [#B_draft]_


This footnote doesn't really make sense anymore. We have the same footnote, named B_draft internally that describes what the fully ratified B extension entails and then also refer to the same footnote for the scalar crypto one. Let's have one footnote each or just leave them away, since the scalar crypto is already clear on its own and if we clarify the bitmanip above already, I also don't think we need a footnote specifying the subextensions anymore.

SamuelRiedel · 2026-07-20T11:26:08Z


 Bit-Manipulation Extension
-  Support for the `RISC-V Bit-Manipulation Extension version 1.0.0 <https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf>`_ and `draft version 0.93 from January 10, 2021 <https://github.com/riscv/riscv-bitmanip/blob/master/bitmanip-0.93.pdf>`_ is optional. [#B_draft]_
+  Support for the `RISC-V Bit-Manipulation Extension version 1.0.0 <https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf>`_, together with the Zbkb and Zbkx sub-extensions of the ratified Scalar Cryptography Extension, is optional. [#B_draft]_


Again, outdated footnote name

SamuelRiedel · 2026-07-20T11:26:31Z

-  The implementation of the Bit-Manipulation Extension comes with an area overhead of 2.7 kGE for the balanced version, 6.1 kGE for the OTEarlGrey version, and 7.5 kGE for the full version.
-  These numbers were obtained by synthesizing the design with Yosys and relaxed timing constraints.


Do we have any new numbers for those? Even if we could simply estimate them?

SamuelRiedel · 2026-07-20T12:01:11Z

+  uint32_t misa_val = processor->get_csr(CSR_MISA);
+  if (rv32b_enabled) {
+    misa_val |= 1 << 1;
+  }
+  processor->get_state()->csrmap[CSR_MISA] =
+      std::make_shared<const_csr_t>(processor.get(), CSR_MISA, misa_val);
+


Why is the B extension a special case here where we need to parse the ISA string ourselves and inject it into the MISA register? Shouldn't this be handled the same way as all the other extensions?

SamuelRiedel · 2026-07-20T15:27:32Z

-              end
              5'b0_1001,                                                              // bclri
              5'b0_0101,                                                              // bseti
              5'b0_1101: illegal_insn = (RV32B != RV32BNone) ? 1'b0 : 1'b1;           // binvi


These instructions require instr[25] to be zero as well in RV32, which is never checked, right?

SamuelRiedel · 2026-07-20T15:30:36Z

        if ({instr[26], instr[13:12]} == {1'b1, 2'b01}) begin
-          illegal_insn = (RV32B != RV32BNone) ? 1'b0 : 1'b1; // cmix / cmov / fsl / fsr
+          illegal_insn = 1'b1; // cmix / cmov / fsl / fsr removed


Do we still need this? They will simply fall under the default case later, right? I think the comment will be a bit confusing if someone looks at this without the context of the bitmanip just being cleaned up.

SamuelRiedel · 2026-07-20T15:46:25Z

-              default: ;
-            endcase
-          end
+          // Ternary bitmanip ops (cmix/cmov/fsl/fsr) removed; these encodings are illegal.


Here we again have an empty if block and the else only contains a switch case with legal encodings, so we could directly use the switch case only, no?

SamuelRiedel · 2026-07-20T15:51:36Z

@@ -124,174 +116,30 @@ package ibex_tracer_pkg;
  parameter logic [31:0] INSN_BEXT = { 7'b0100100, 10'h?, 3'b101, 5'h?, {OPCODE_OP} };

  // ZBP


Mention of old extension

SamuelRiedel · 2026-07-20T15:52:34Z

+    // The generalized-reverse / or-combine butterfly below implements the ratified
+    // subset only: rev8  and orc.b.

    logic [4:0] zbp_shift_amt;


Mention of old extension in variable name

thommythomaso self-assigned this Jul 13, 2026

thommythomaso requested a review from SamuelRiedel July 13, 2026 12:51

thommythomaso force-pushed the tbenz/bitmanip branch 2 times, most recently from d0b04a1 to 359815f Compare July 14, 2026 15:23

thommythomaso added 19 commits July 14, 2026 17:56

[rtl] CSR MISA string X to B

07ff056

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove Xzbe0p93

9b9d172

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove Xzbf0p93

0c5d7d0

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove Xzbr0p93

571dfbc

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove Xzbt0p93

853b0e9

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove slo sloi sro sroi from Xzbb0p93

004cd48

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove packu from Xzbb0p93

59c2213

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove xpermh from Xzbb0p93

89a1cb1

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove grev Xzbb0p93

141920a

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove gorc from Xzbb0p93

b5464a6

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Drop the RV32BOTEarlGrey enum from the ibex_pkg

5cb072f

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Move and rename xperm4 and xperm8 from Xzbp0p93 to Zbkx

a6e4d49

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Move and rename zip and unzip from Xzbp0p93 to Zbkk

25ef875

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Move and rename orgb from Xzbp0p93 to Zbb

2491298

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Move and rename rev8 and brev8 from Xzbp0p93 to Zbkb

082c959

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Support pseudo instruction zexth

71e83b0

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Complete Zbkb

7dab369

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Lint fix

4005e9e

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[rtl] Remove unsupported RV32BOTEarlGrey

242bc59

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

thommythomaso force-pushed the tbenz/bitmanip branch from 359815f to 242bc59 Compare July 14, 2026 15:56

[dv] Fix spike cosim

6062faa

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

thommythomaso force-pushed the tbenz/bitmanip branch from afac420 to 6062faa Compare July 15, 2026 12:38

thommythomaso mentioned this pull request Jul 15, 2026

[ibex, rtl] Move to ratified bitmanip extensions lowRISC/opentitan#30502

Draft

marnovandermaas reviewed Jul 15, 2026

View reviewed changes

thommythomaso marked this pull request as ready for review July 16, 2026 06:53

SamuelRiedel reviewed Jul 20, 2026

View reviewed changes

		{7'b000_0100, 3'b100}, // pack
		{7'b000_0100, 3'b111}, // packh

		The implementation of the Bit-Manipulation Extension comes with an area overhead of 2.7 kGE for the balanced version, 6.1 kGE for the OTEarlGrey version, and 7.5 kGE for the full version.
		These numbers were obtained by synthesizing the design with Yosys and relaxed timing constraints.

		@@ -124,174 +116,30 @@ package ibex_tracer_pkg;
		parameter logic [31:0] INSN_BEXT = { 7'b0100100, 10'h?, 3'b101, 5'h?, {OPCODE_OP} };

		// ZBP

Uh oh!

Conversation

thommythomaso commented Jul 13, 2026

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thommythomaso commented Jul 15, 2026

Uh oh!

SamuelRiedel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants